-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix Expo SDK 54 compatibility #3597
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 55e1e8b:
|
|
It looks like this would break compatibility with Expo Go, at least for now. What is your estimation on how breaking this change is? |
|
Would my workaround be sufficient for us to not ship a breaking change? #3583 (comment) |
|
@CodyJasonBennett That would still be a breaking change as well, since @krispya I chose this approach because it felt more future-proof. Personally, I don’t see development builds as a blocker for adopting a library (more details here: https://github.com/expo/fyi/blob/main/expo-go-usage.md), especially since the underlying issue has already been fixed upstream and just needs to ship in the next Expo Go release. That said, I’m happy to hold off if you’d prefer to wait until the fixed Expo Go version is available |
|
Why would it be a breaking change? Does it not work? The whole point is to conditionally import, which allows us to support either subpath or either version. My hope is that it could be a quick fix we can ship until we drop the legacy API in the next major. |
|
Sorry, I misread your suggestion and didn't realize it was a conditional import rather than a direct change. I haven't tested this approach yet, but I assume it should work |
|
@CodyJasonBennett I tested your suggestion, but encountered an error requiring |
Fixes the
TypeError: Cannot read property 'Base64' of undefinederror on Expo SDK 54, caused byexpo-file-system/nextbecoming the default export and the old API moving toexpo-file-system/legacy(see #3583).Migrated the polyfill to use the new
expo-file-systemAPI.Notes
expo-file-systemon iOS (see expo/expo#40287)expo-file-systemversions (pre-SDK 54)